Expose the pro auto-renewing config key to Kotlin - #49
Open
mpretty-cyro wants to merge 7 commits into
Open
Conversation
libsession PR #121 adds the `A` / auto_renewing user-profile key, but it is core-only — this wrapper had no binding for it, so clients could not read or write it. Adds the JNI pair and the Kotlin declarations, following the existing ProAccessExpiry shape. The accessor is presence-only and the doc comment says so at the API boundary rather than only at the call site: set_pro_auto_renewing(false) ERASES the key, so `false` and "never written" are the same state through this getter. That is #121's encoding surfacing here, not something introduced by the binding, and it is where the next reader will meet it. Pins the libsession-util submodule to 8e5634b8, the head of #121, which is UNMERGED — so this commit cannot merge until #121 does. It is also not the identically-subjected pro-auto-renewing-config-pfs commit, which rebases the same change onto the PFS track. Verified in the built APK across all four ABIs rather than by a successful compile: a JNI signature mismatch is invisible at compile time and only traps at runtime.
The access expiry the backend sends is grace-INCLUSIVE, so it is coverage end and the paid-through instant is expiry - grace. Clients need the grace period in synced config to compute that at all: it drives the renewal date, the grace indicator, and the startup gate's decision about whether a renewal is overdue. Adds the JNI pair and Kotlin declarations for config key `G`, matching the shape of the auto-renewing pair beside it. Crosses the boundary as seconds and is presented as a Duration. No presence check, unlike `A`: the backend sends 0 whenever the subscription is not auto-renewing, so unset and zero describe the same account and both give `expiry - 0`. There is no state a caller could act on differently, so a predicate would copy the shape of the auto-renewing accessor without its reason. Re-pins the libsession-util submodule from 8e5634b8 to 269f8b88, which contains it plus the grace key. Still UNMERGED, so this cannot merge until that does. All four JNI symbols verified present across all four ABIs in the built APK: a signature mismatch is invisible at compile time and only traps at runtime.
The proof response carries the account's grace period and whether it auto-renews, so a client refreshing its cached access expiry from a proof can keep all three coherent. Neither field reached Kotlin: core only began parsing them in the commit this re-pins to. Both cross the JNI boundary as value + presence pairs and surface as nullable Kotlin properties, matching the hasLatestPayment shape already used for get-pro-status. That is not decoration: absent must stay distinguishable from zero/false. The client writes these into presence-only config keys where writing false or zero ERASES them, so an older backend -- which sends neither field -- would otherwise have every proof fetch wipe a value correctly learned from get_pro_status. Re-pins libsession-util 269f8b88 -> aa52b3ee for the parse. Still unmerged. Verified: the four user_profile symbols present across all four ABIs in the built APK, the new Kotlin properties present in the dex, and the JNI constructor descriptor cross-checked against the Kotlin signature by hand -- (L..;L..;JZJZZ)V both sides. A descriptor mismatch would be invisible at compile time and throw on the first parse.
… optional Core amended these from optionals with presence flags to plain required values: no backend predates them -- Pro has not shipped -- so the absent case does not arise. Drops the presence half of the JNI constructor descriptor and the nullable folding on the Kotlin side. The Kotlin surface is non-nullable for both, which is the honest type. A nullable that can never be null invites `?: false` or `?: Duration.ZERO` at call sites, and neither default is inert: writing false to a presence-only config key ERASES it. Requiring the fields means a malformed response fails the parse and the client keeps what it has, rather than persisting a default. accountExpiry stays nullable with its 0-sentinel, because that field genuinely is absent on some outcomes. The asymmetry inside one class is real rather than an oversight, and each property says which it is. Re-pins libsession-util aa52b3ee -> 799f1972. Verified: the four user_profile symbols present across all four ABIs in the built APK; the presence-flag names absent from the dex; and the JNI constructor descriptor cross-checked by hand against the Kotlin signature AGAIN, because dropping two parameters changes it -- (L..;L..;JJZ)V both sides, was (L..;L..;JZJZZ)V.
…ype no longer can Re-pins libsession-util 799f1972 -> f197a0bd and corrects the doc comments, which had the right nullability for the wrong reason. Removing the optionals did not remove the absent case; it collapsed it into a value. The parser returns on the failure path before filling these two, so on every non-OK outcome they hold struct defaults -- grace 0, renewing false -- and the C struct carries no presence flag, so nothing distinguishes that from a backend genuinely saying "no grace, not renewing". Which matters because the client writes them into presence-only config keys where false ERASES. For subscription_expired, not_subscribed and revoked that erasure is truthful; for a protocol error or a transport failure it would wipe a flag get_pro_status had correctly learned, on the strength of a response that said nothing about the account. The previous comment claimed false was truthful for "the failure outcomes", which holds for exactly three error codes. So both properties now say: only meaningful on a successful proof, read inside a success branch or not at all. Non-nullable is still right -- core does not model absence, and a nullable that can never be null invites the collapse it was meant to prevent -- but the protection moved from the type to the call site's placement rather than disappearing.
Clearing the access expiry now clears the auto-renewing flag with it, alongside the grace period it already cleared. A behaviour change in core, not in this wrapper: no binding changes. It fixes an asymmetry where a revoked or cleared subscription left a stale renewing flag behind. The three keys were previously coherent only because every consumer happens to test the expiry before reading the flag -- true on all three clients and enforced by nothing. Maintaining the invariant on the write side is what stops the next consumer inheriting the assumption without knowing it exists. Android reads the flag and the grace period in exactly one place, and it tests the expiry first, so this changes nothing here. The check stays -- it is a necessary "never subscribed" branch in its own right -- but it is no longer what keeps the keys coherent.
mpretty-cyro
marked this pull request as ready for review
August 10, 2026 06:56
`G` is how much longer the account is served PAST the expiry, so coverage ends at `E + G`. The docs said the backend folded grace into the expiry and that `E - G` recovered a paid-through instant; that fold was removed upstream and subtracting now double-counts. Also names the collision the KDoc was silent about: ProPaymentItem and GetProStatusResponse both have a `gracePeriod`, and only the account-level one answers coverage questions. The payment-level field is the raw store value and is not gated on auto-renewing, so a cancelled subscriber can carry a multi-day value in it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Expose the pro auto-renewing and grace-period keys, and the proof response's advisory fields
LibSession-Util-Android@feature/pro-auto-renewing-glue→ basemain(a00aa4a).Two user-profile JNI pairs (
AandG), the proof response's two advisory fields, and a submodulepin bump.
🔴 Cannot merge until libsession-util libsession-util#121 merges
The
libsession-utilsubmodule is pinned tob066ba27, which is UNMERGED. It contains#121 (
8e5634b8, theA/auto_renewingkey — an open PR), theG/ grace-period key, and the proof-response parse foraccount_auto_renewingandaccount_grace_period_duration, both required on a successful proof. Merging this wrapper firstwould pin it to a commit that isn't on
dev.A reviewer looking at a glue diff has no reason to have libsession-util#121 in mind, so it is stated here as well as
in the commit message.
Not the
-pfsbranch.JasonFork/pro-auto-renewing-config-pfscarries an identical commitsubject with a different sha and is not a descendant — it rebases the same change onto the PFS
track, dragging that line in with it.
gh pr view 121 --json headRefOidconfirms8e5634b8.Merge order: libsession-util#121 → this → a
LibSession-Util-Androidrelease → the Android client'slibsessionUtilAndroidVersionbump → the client PR (session-androidfeature/pro-status-refresh-unification), which is what needs these accessors.Why this exists
Both keys are added to core only, and this wrapper had no binding for either — "the core change
landed" is not the same as "the accessor is reachable from Kotlin".
A/auto_renewing— the client persists it into synced config beside the access expiry, so alinked device has the account state without its own fetch.
G/ grace period — needed because the access expiry the backend sends is the payment-duedate, and coverage runs a further grace period past it. The backend states the contract at the site
that builds the response: "
expiry_ts+grace_period_durationis exactly when we stop serving".So entitlement ends at
expiry + grace, which is what the grace indicator, the coverage-end wake andthe startup gate all have to key off. Without
Gin config the gate cannot compute it at all.Both follow the existing
ProAccessExpiryshape:extern "C"pair, interface member,externaloverride. No new pattern.
Gcrosses the boundary as seconds (jlong) and is presented as ajava.time.Durationon the Kotlin side, matching how the client already handles the wire value.Known limitation — inherent to
A, surfaced hereAis presence-only.set_pro_auto_renewing(false)erases the key rather than storing zero,so through this accessor a stored
falseand "never written" are indistinguishable —getProAutoRenewing()returnsfalsefor both.That is libsession-util#121's encoding, not something this binding introduces, but this is where a Kotlin caller
meets it, so it is documented on the accessor itself rather than only at the call sites. Callers that
need to tell the two apart cannot, through this API.
Not included: a mirror of
user_profile_has_pro_auto_renewing(). That predicate was proposed andthen withdrawn upstream as vacuous — under this encoding
Ais present if and only if its value is1, so it would have returned the same bit as the plain getter in every reachable state. Nothing here
ever depended on it.
Verification — and what it does not rest on
A JNI signature mismatch is invisible at compile time and traps at runtime, so a green build is
not evidence for a change of this kind, and a reviewer cannot see the binding is correct from the
diff. Checked against the built artefact instead —
app-play-debug.apk, all four ABIs:All four symbols present in every ABI —
getProAutoRenewing,setProAutoRenewing,getProGracePeriodSeconds,setProGracePeriodSeconds. APK built from this branch with the consumingclient change and checked immediately after.
:app:assemblePlayDebuggreen; the client's suite is 219tests / 0 failures against it.
The leg not run: there is no unit test for the binding in this repo. No JVM test here can load
libsession_util.so, which is why the evidence is a symbol check on the real artefact rather than atest. Stating it rather than letting "verified" cover a check that wasn't possible.
Reviewer notes
jboolean/jlongpairs rather than anintwith a sentinel.On Android the natural binding — mirroring the accessor directly above it — would be
static_cast<jboolean>(-1)=255=JNI_TRUE, so "never stored" would silently reach Kotlin as"auto-renewing".
jbooleanisuint8_t; the narrowing is legal and produces no diagnostic.t/Tbump on either write path: both are backend-derived state likeE,IandR, not userprofile edits, and libsession omits the bump for them deliberately.
Gneeds no presence check, unlikeA. The backend sends grace = 0 whenever the subscription isnot auto-renewing, so "unset" and "zero" describe the same account and both give
expiry + 0. Thereis no state a caller could act on differently, so adding a predicate by symmetry with
Awould becopying the shape without the reason.
Core's parser returns on the failure path before filling them, so every non-OK outcome yields grace 0
and renewing false from struct defaults — no presence flag, non-nullable type, nothing to trip over.
Since the client writes them into presence-only config keys where
falseerases, a read outside asuccess branch can wipe a flag
get_pro_statushad correctly learned. Both properties say so. Theprotection is placement, not the type.
accountExpiryis not. That asymmetry inside oneclass is deliberate: the two new fields are required on a successful proof, so a nullable type would
be a lie inviting
?: false— andfalsewritten to a presence-only key erases it.accountExpirykeeps its 0-sentinel because it genuinely is absent on some outcomes.